feat(tools): one static /mcp endpoint, session chosen by bearer key - #44
Merged
Merged
Conversation
A workspace MCP config is shared by every session in a pod, so a URL that names one session routes all of them to that session's computer (seen 2026-09-29 on kiro-1040: agent-black, agent-rp1 and mac all answered from macmini). Sessions now also get OPENAB_TOOLS_MCP_ENDPOINT (http://127.0.0.1:<port>/mcp, identical everywhere) and OPENAB_TOOLS_MCP_TOKEN (the same key as in the URL). POST /mcp + Authorization: Bearer routes by key, with a constant-time non-short-circuit scan; missing, malformed and unknown keys are one 401. /mcp/{session}/{key} is unchanged. The contract documents the one-file config: Kiro expands ${VAR} in headers, not in url.
This was referenced Oct 1, 2026
thepagent
pushed a commit
that referenced
this pull request
Oct 1, 2026
…ok (#49) * feat(tools): wire the tools MCP into the CLI from an image startup hook (#39) The runtime keeps knowing no coding CLI's config format. Instead it gains one generic seam, `--startup-hook <FILE>`: an image-provided executable it runs once, after seeding and before serving, best-effort (failure, non-zero exit or a 30 s timeout is logged and serving continues). After seeding matters: a seed archive may carry the very config file the hook edits. The images ship deploy/hooks/startup-hook.sh (bash + jq; jq added to both Dockerfiles). For kiro-cli, when PTY_TOOLS_LISTEN is set, it writes #44's session-independent form into ~/.kiro/settings/mcp.json: computer = { url: http://<PTY_TOOLS_LISTEN>/mcp, headers: { Authorization: "Bearer ${OPENAB_TOOLS_MCP_TOKEN}" } } and adds @computer/* to allowedTools in existing agent files. No session name or key is ever on disk, so one shared workspace config serves every session and survives every key rotation. With the tools plane off, only the entry the hook wrote is withdrawn. A new CLI is a function in the hook, not a runtime change. Merge rules come from #45: other servers/settings preserved, wrong-shaped or malformed files left byte-identical, agent files never created, dangling symlinks left alone, atomic writes that keep the file's mode. Changed from #45: no stdio JS bridge (superseded by #44's header route), restrictive `tools` lists and `mcpServers` in agent files are not touched, key order is kept, and the knowledge lives in the image rather than the runtime. Closes #39. Supersedes #45. Co-authored-by: Reese-max <198568376+Reese-max@users.noreply.github.com> * fix(hook): scrub the hook's env, kill and reap its group, respect user entries Review round 1 on #49. - B1: the hook ran with the runtime's full environment while jq loaded ~/.jq from the shared workspace, so a planted ~/.jq could write e.g. an ECS credentials URI into a config the next session reads. The hook now gets env_clear() + the session allowlist + OPENAB_PTY_TOOLS_LISTEN, and the script runs jq with HOME=/nonexistent. - S1: the hook leads its own process group; on exit or timeout the group is SIGKILLed and reparented members are reaped (waitpid(-pgid)), so no orphan or zombie outlives it. - S2: main.rs documents why the exec'd, dumpable hook is safe (scrubbed env, no session yet). Inline step numbers fixed. - S3: the hook runs after the binds and is handed the tools address the runtime actually bound (same as OPENAB_TOOLS_MCP_ENDPOINT), not PTY_TOOLS_LISTEN re-read from env. IPv6 tested. - S4: tools-on replaces `computer` only when absent, ours (by header), or a hand-wired URL on this listener; anything else is left with a warning. - S5: a failed temp-file write is never renamed over the config. - Nits: non-regular paths and multi-document files left alone; HOME unset exits 0; "*" in allowedTools counts as trusted; WaitFailed outcome; jq version noted in the Dockerfile; `jq --version` in CI; tests for all of the above (39 shell cases, 8 hook tests incl. children and background members). Test-only ETXTBSY retry for scripts exec'd right after writing. * fix(hook): never fatal on a JoinError, tolerate non-UTF-8 env at boot Review round 2 nits on #49. - A failed spawn_blocking task (panic inside it) was propagated with `?`, contradicting the hook's best-effort contract; it is now a warning. - std::env::vars() panics on a non-UTF-8 variable, which with --startup-hook would have aborted the runtime at boot. Use vars_os() and drop variables that are not UTF-8 (the allowlist is ASCII anyway). Verified: runtime started with BAD=\xff\xfe in its env and a hook, hook ran and saw OPENAB_PTY_TOOLS_LISTEN, runtime kept serving. --------- Co-authored-by: Reese-max <198568376+Reese-max@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Refs #39.
Problem. A workspace MCP config is shared by every session in a pod, so it cannot name one session in its URL. On kiro-1040 three sessions (
agent-black,agent-rp1,mac) were each attached to a different computer, yet all answeredsys_infofrom macmini, because the sharedmcp.jsonpinned/mcp/mac/<key>. Kiro does not expand${VAR}insideurl(tested), but it does insideheaders.Change.
OPENAB_TOOLS_MCP_ENDPOINT(http://127.0.0.1:<port>/mcp, identical in every session) andOPENAB_TOOLS_MCP_TOKEN, the same 64-hex key as in the URL.POST /mcpwithAuthorization: Bearer <key>routes to whichever session owns the key. The lookup is a constant-time, non-short-circuit scan. Missing, malformed or unknown keys all get one401withWWW-Authenticate: Bearer.GETreturns405, as before./mcp/{session}/{key}is unchanged.url+headers.Authorization: Bearer ${OPENAB_TOOLS_MCP_TOKEN}) as the preferred wiring.Tests. New e2e test
one_static_endpoint_routes_each_session_by_its_bearer_key: two sessions hit the same endpoint with their own keys and each gets its own fake computer (A/B). Missing, malformed, zero and truncated keys return401. A deleted session's key returns401while the other session is unaffected. On macmini: 171 unit tests and 8/8 e2e (--ignored) pass; clippy-D warningsand fmt are clean.